Hi!

Thank you so much!😊 I'm out on vacation right now, so the review probably won't come until next week some time.

Regarding the errors, it's a bug, we haven't been good enough at testing B2C tokens with the new user model.
B2C tokens probably don't have a tid and oid claim.

The fix is to make the tid and oid optional fields.
I wish we could have a better solution for separate models for B2C, but Python typing system is a bit hard to work with when you return a union of multiple models, and require the developer to isintance() check which model it is at all times - a bad developer experience.

CC @omBratteng @enadeau

I have never worked with B2C tokens, but I see Microsoft has a list of the claims in a B2C token.

Thanks for your quick reply 👍

I could reduce the amount of validation errors to only choke on tid by including the User ID as a claim.

But with tid it's trickier without touching the fastapi-azure-auth library. Would it make sense to add a simple PR to make tid optional in the Pydantic model for User?

Yes, exactly, a PR would be perfect! Both should be optional 😊

Codecov Report

Merging #140 (2c05d06) into main (f6605a5) will not change coverage.
Report is 5 commits behind head on main.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #140   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files           6        6           
  Lines         248      247    -1     
=======================================
- Hits          248      247    -1     

see 1 file with indirect coverage changes

@JonasKs this PR is ready for review. With recent version 4.2.1 I was able to log in as per steps in the new B2C documentation.

Thank you for your feedback - happy it works😊 This deserve a proper review where I try it out, so I'll make some time next week to do so.

hi there, did you find the time to take a look at it? disclaimer, I've been using it successfully during development and deployments but have not yet pushed to real users yet.

hi there, did you find the time to take a look at it? disclaimer, I've been using it successfully during development and deployments but have not yet pushed to real users yet.

Thanks @davidhuser for this amazing work. I'm excited to see this feature @JonasKs

I'm so sorry, I've struggled to find time, but I should have time next week 😊

I'm so sorry, I've struggled to find time, but I should have time next week 😊

I'm excited to see this feature @JonasKs
How can I help ?

I'll look at this today, but please note this isn't a feature, it's documentation. You can still use it, even if it's not merged.

I'll look at this today, but please note this isn't a feature, it's documentation. You can still use it, even if it's not merged.

Oh sorry I haven't see that claims tid and oid are now optional and merge are done.
Thank you

Correct 😊

This looks very good, thank you so much for the time and effort 🙏 Sorry for the late review.

I'll merge when you accept / resolve threads. 😊

threads are resolved / suggestions accepted, so if the checks are not a problem we can merge 🙂

Thank you so much 😊